Don't recompile git deps so frequently

The previous logic for recompiling any dependency was almost entirely based on
the mtimes of the relevant input files. This isn't quite what's desired for git
and registry dependencies because the mtime could be fluctuating while the files
aren't changing. For example:

1. Project A checks out git repo C at revision C1
2. Project A builds, records mtimes of C
3. Project B checks out git repo C at revision C2
4. Project B builds, records new mtimes of C
5. Project A is rebuilt, rebuilding C b/c mtimes are different

In step 5 here C should not be rebuilt because the revision didn't actually
change.

This commit alters git/registry dependencies to completely bypass the --dep-info
emitted and only rely on the git/registry source to inform what the fingerprint
is. This is the revision/version, respectively, and should be all that's
necessary to track changes to the repo and trigger a rebuild.

diff --git a/src/cargo/ops/cargo_rustc/fingerprint.rs b/src/cargo/ops/cargo_rustc/fingerprint.rs
index aebb93ff60cf76604dc357f0953f17f0c84de694..228dae234795a41ef9aa70e783bb5a82ef6bb38c 100644 (file)
--- a/src/cargo/ops/cargo_rustc/fingerprint.rs
+++ b/src/cargo/ops/cargo_rustc/fingerprint.rs
@@ -2,7 +2,7 @@ use std::hash::{Hash, Hasher};
 use std::hash::sip::SipHasher;
 use std::io::{fs, File, UserRWX, BufferedReader};
 
-use core::{Package, Target};
+use core::{Package, Target, PathKind};
 use util;
 use util::hex::short_hash;
 use util::{CargoResult, Fresh, Dirty, Freshness, internal, Require, profile};
@@ -47,18 +47,31 @@ pub fn prepare_target(cx: &mut Context, pkg: &Package, target: &Target,
     let filename = filename(target);
     let old_loc = old.join(filename.as_slice());
     let new_loc = new.join(filename.as_slice());
-    let doc = target.get_profile().is_doc();
+
+    // We want to use the package fingerprint if we're either a doc target or a
+    // path source. If we're a git/registry source, then the mtime of files may
+    // fluctuate, but they won't change so long as the source itself remains
+    // constant (which is the responsibility of the source)
+    let use_pkg = {
+        let doc = target.get_profile().is_doc();
+        let path = match pkg.get_summary().get_source_id().kind {
+            PathKind => true,
+            _ => false,
+        };
+        doc || !path
+    };
 
     debug!("fingerprint at: {}", new_loc.display());
 
     // First bit of the freshness calculation, whether the dep-info file
     // indicates that the target is fresh.
     let (old_dep_info, new_dep_info) = dep_info_loc(cx, pkg, target, kind);
-    let are_files_fresh = doc || try!(calculate_target_fresh(pkg, &old_dep_info));
+    let are_files_fresh = use_pkg ||
+                          try!(calculate_target_fresh(pkg, &old_dep_info));
 
     // Second bit of the freshness calculation, whether rustc itself and the
     // target are fresh.
-    let rustc_fingerprint = if doc {
+    let rustc_fingerprint = if use_pkg {
         mk_fingerprint(cx, &(target, try!(calculate_pkg_fingerprint(cx, pkg))))
     } else {
         mk_fingerprint(cx, target)

diff --git a/tests/test_cargo_compile_git_deps.rs b/tests/test_cargo_compile_git_deps.rs
index 90e66ca7a52bc7577e049b03216de8cbc80f21a4..91fb2e1a7696aa9903d3608257dfb2af858211c9 100644 (file)
--- a/tests/test_cargo_compile_git_deps.rs
+++ b/tests/test_cargo_compile_git_deps.rs
@@ -1080,3 +1080,67 @@ test!(git_name_not_always_needed {
 {compiling} foo v0.5.0 ({url})
 ", updating = UPDATING, compiling = COMPILING, url = p.url(), bar = p2.url())));
 })
+
+test!(git_repo_changing_no_rebuild {
+    let bar = git_repo("bar", |project| {
+        project.file("Cargo.toml", r#"
+            [package]
+            name = "bar"
+            version = "0.5.0"
+            authors = ["wycats@example.com"]
+        "#)
+        .file("src/lib.rs", "pub fn bar() -> int { 1 }")
+    }).assert();
+
+    // Lock p1 to the first rev in the git repo
+    let p1 = project("p1")
+        .file("Cargo.toml", format!(r#"
+            [project]
+            name = "p1"
+            version = "0.5.0"
+            authors = []
+            build = 'true'
+            [dependencies.bar]
+            git = '{}'
+        "#, bar.url()).as_slice())
+        .file("src/main.rs", "fn main() {}");
+    p1.build();
+    p1.root().move_into_the_past().assert();
+    assert_that(p1.process(cargo_dir().join("cargo")).arg("build"),
+                execs().with_stdout(format!("\
+{updating} git repository `{bar}`
+{compiling} bar v0.5.0 ({bar}#[..])
+{compiling} p1 v0.5.0 ({url})
+", updating = UPDATING, compiling = COMPILING, url = p1.url(), bar = bar.url())));
+
+    // Make a commit to lock p2 to a different rev
+    File::create(&bar.root().join("src/lib.rs")).write_str(r#"
+        pub fn bar() -> int { 2 }
+    "#).assert();
+    bar.process("git").args(["add", "."]).exec_with_output().assert();
+    bar.process("git").args(["commit", "-m", "test"]).exec_with_output()
+       .assert();
+
+    // Lock p2 to the second rev
+    let p2 = project("p2")
+        .file("Cargo.toml", format!(r#"
+            [project]
+            name = "p2"
+            version = "0.5.0"
+            authors = []
+            [dependencies.bar]
+            git = '{}'
+        "#, bar.url()).as_slice())
+        .file("src/main.rs", "fn main() {}");
+    assert_that(p2.cargo_process("build"),
+                execs().with_stdout(format!("\
+{updating} git repository `{bar}`
+{compiling} bar v0.5.0 ({bar}#[..])
+{compiling} p2 v0.5.0 ({url})
+", updating = UPDATING, compiling = COMPILING, url = p2.url(), bar = bar.url())));
+
+    // And now for the real test! Make sure that p1 doesn't get rebuilt
+    // even though the git repo has changed.
+    assert_that(p1.process(cargo_dir().join("cargo")).arg("build"),
+                execs().with_stdout(""));
+})